Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't accept type wrapper in index request #4484

Closed
clintongormley opened this issue Dec 17, 2013 · 5 comments
Closed

Don't accept type wrapper in index request #4484

clintongormley opened this issue Dec 17, 2013 · 5 comments

Comments

@clintongormley
Copy link

Currently it is possible to index a document as:

POST /myindex/mytype/1
{ "foo"...}

or as:

POST /myindex/mytype/1
{
    "mytype": {
        "foo"...
    }
}

This makes indexing non-deterministic and fields can be misinterpreted as type names.

We should accept only the first form, ie without the type wrapper.

@kimchy
Copy link
Member

kimchy commented Jan 7, 2014

I have concerns about this change... . The reason why it was supported to begin with was that some serialization tools that convert objects to json objects tend to sometime also include a top level "type" in it... (not many, I admit...).

I am also concerned about the backward comp. questions that will happen here..., I know we can break 1.0 backward comp., but this can be very annoying (think about users relaying on it, and now they will need to somehow strip the top level).

I agree that currently, its annoying, because if a json document has the first field name the same as the type, its very annoying since it breaks badly. Which I think this is where this change is coming from.

But, at the end, if we can support the above, if there is a single top level object, and thats the same as the type name, then we treat it as the type, and parse the fields within it. If its not (its a field, or there is another object at the same level), then treat it as a "regular" document. If we manage to do this, then I would say its better.

@clintongormley
Copy link
Author

I have never seen anybody gist an example where they index a document with the body wrapped with the type name. I have however seen a few people have issues because of the ambiguity introduced by supporting optional type wrapping. Better heuristics may make the issue less frequent, but won't solve it completely.

I'd vote for pushing the change as it is or, alternatively, adding a setting which allows the user to continue using the old behaviour should they so choose, but disable it by default.

@dakrone
Copy link
Member

dakrone commented Jan 9, 2014

I agree with Clint also, I've never seen anyone index documents wrapped by the type name, and trying to determine whether the outer object is a type-name or object-name will be confusing to the end user since the decision is hidden inside internal ES logic. I think we should remove the wrapping option. What do you think @kimchy?

@kimchy
Copy link
Member

kimchy commented Jan 11, 2014

I am ok with putting this behind a setting (can be an index based setting even, @dakrone, the Builder gets indexSettings), and the setting can default to not allow it.

@dakrone
Copy link
Member

dakrone commented Jan 11, 2014

Okay, I'll add a setting for this so it can be enabled/disabled.

dakrone added a commit to dakrone/elasticsearch that referenced this issue Jan 13, 2014
Currently it is possible to index a document as:

```
POST /myindex/mytype/1
{ "foo"...}
```

or as:

```
POST /myindex/mytype/1
{
    "mytype": {
        "foo"...
    }
}
```

This makes indexing non-deterministic and fields can be misinterpreted
as type names.

This changes makes Elasticsearch accept only the first form by default,
ie without the type wrapper. This can be changed by setting
`index.mapping.allow_type_wrapper` to `true`` when creating the index.

Closes elastic#4484
brusic pushed a commit to brusic/elasticsearch that referenced this issue Jan 19, 2014
Currently it is possible to index a document as:

```
POST /myindex/mytype/1
{ "foo"...}
```

or as:

```
POST /myindex/mytype/1
{
    "mytype": {
        "foo"...
    }
}
```

This makes indexing non-deterministic and fields can be misinterpreted
as type names.

This changes makes Elasticsearch accept only the first form by default,
ie without the type wrapper. This can be changed by setting
`index.mapping.allow_type_wrapper` to `true`` when creating the index.

Closes elastic#4484
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants